All committers have signed the CLA.

Hi @rpavlik, here is loader patch of API layer discovery, please help review, thanks!

An issue (number 2045) has been filed to correspond to this pull request in the internal Khronos GitLab (Khronos members only: KHR:openxr/openxr#2045 ), to facilitate working group processes.

This GitHub pull request will continue to be the main site of discussion.

I did import this into OpenXR gitlab for internal discussion: https://gitlab.khronos.org/openxr/openxr/-/merge_requests/2888

If you can't access that you might need one of your colleagues to assist, since I know they don't let everyone there make a Khronos account.

I've taken an initial pass over this with a few minor pieces, I want to double check the layer ordering as I think runtime shipped layers should be the lowest in the stack (just above the runtime) which I believe this is currently doing correctly but I want to double check.

Yes, I can confirm this is exactly this patch will do. Runtime layers will be loaded above the chosen runtime, and thanks for the suggestions, I will modify patch accordingly.

Updates (9/27):

1. Functions/instance_extensions query pattern should be revised, better to use similar pattern as functions query in runtime discovery.(loader/broker/runtime side) ---> done

• Following paths will be used:

  • /openxr/[major_ver]/abi/[abi]/runtimes/active (existing)
  • /openxr/[major_ver]/abi/[abi]/runtimes/[package_name]/functions (existing)
  • /openxr/[major_ver]/abi/[abi]/api_layers/[implicit|explicit] (new in this MR - rename from api_layer )
  • /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/functions (new in this MR)
  • /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/instance_extensions (new in this MR)

• The proceedure for API layers would then be:

  • Query active implict/explict layers by the /openxr/[major_ver]/abi/[abi]/api_layers/[implicit|explicit] path.

  • For each layer get it's package_name and layer_name and check has_functions and has_instance_extensions columns.

  • If has_functions is true, then query function mappings with /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/functions

  • If has_instance_extensions is true, then query extensions and versions with /openxr/[major_ver]/abi/[abi]/api_layers/[package_name]/[layer_name]/instance_extensions

2. Adapt the protocol so that the existing authorities for the brokers are used, and so that both the system broker and installable broker can report API layers.(loader/broker/runtime side) ---> done
3. Remove ‘enableEnvironment’/‘disableEnvironment’ in API layers as they can have no effect on Android as EnvVars are not supported.(loader/broker/runtime side) ---> done
4. Multiple cases (case A, B, D, E, F1/F2/F3) in runtime/API layer discovery are not needed or have potential security issues to achieve ‘same-origin’ policy. (mainly loader side) ---> WIP

12/7/2023: rebase code base to OpenXR SDK 1.0.32.1 hotfix: Fix CMake build of loader on Universal Windows Platform

Rebase to latest: 99256e9 (3/22)

Rebase to latest: 9e9d023ffcc67037ee244f6f0bc3785370814c96. (4/18/2024)

• Rebase to loader release 1.1.36.

• Add "disable_environment" and "enable_environment" back on Android platform.

OK! I was just importing this to GitLab, and I got a little carried away because I saw a few things to fix...

Anyway it is now a lot cleaner of a PR, less code duplication applied, etc, and there are individual commits for small atomic changes. Additionally, there are a handful of changes that are not dependent on the layer work and can be merged on their own.

I dropped the part of the change that cached the results of getting the API layers, as that seemed premature and probably unwise given that there are two authorities they could be pulled from. I did not drop the caching of the runtime chosen itself, but I did split it into a separate commit and I am inclined to drop it, as unrelated, more global state, etc.

This MR works fine (after my adjustments) with the current brokers out there (as in, does not grab any layers, but doesn't fall to pieces) but is a little needlessly loud because of an over-eager exception in the installable broker.

I did add a commit to require disable_sys_prop just like we require disable_environment, not sure if we want that or not. Also not sure if we want to validate the names of those props in any way, enforce a pattern or something?